refactor(core): reorganize UI into component library#121
Conversation
…ayout modules Move UI components from flat structure into organized subdirectories: - prompts/ for user input components (Confirm, Select, TextInput, etc.) - display/ for presentational components - layout/ for structural components (FullScreen, ScrollArea, Tabs) - Consolidate output module into single file - Add screen module with provider and context exports Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
🦋 Changeset detectedLatest commit: 0f85db9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The reduce with a void accumulator was semantically incorrect for side-effect iteration. Revert to map which is the idiomatic choice in this codebase. Co-Authored-By: Claude <noreply@anthropic.com>
Merge origin/main into feat/component-lib, resolving conflicts in: - packages/core/src/index.ts: keep expanded type exports from main + ScreenContext from branch - packages/core/src/ui/output.tsx: adopt cancelled/error spinner states from main using theme abstractions Co-Authored-By: Claude <noreply@anthropic.com>
Merging this PR will not alter performance
Comparing Footnotes
|
Remove all backwards compatibility for the deprecated `spinner` option on `cli()`, `createContext()`, and `createTestContext()`. Consumers must migrate to `status` — no deprecation period, hard break. - Remove `spinner` from CliOptions, RuntimeOptions, CreateContextOptions, TestContextOptions and all pass-through call sites - Remove compat `resolveStatus` logic that wrapped spinner in Status - Remove "backward compatibility" comment on screen re-exports - Delete deprecated-spinner test case - Clean up unused Spinner type imports Co-Authored-By: Claude <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a large UI surface refactor and feature set: new barrel modules (ui/display, ui/layout, ui/prompts, screen), many new Ink-based prompt and display components (Autocomplete, Confirm, GroupMultiSelect, MultiSelect, TextInput, PasswordInput, PathInput, Select, SelectKey, Alert, ProgressBar, StatusMessage, ErrorMessage, Spinner), a centralized theme module, Storybook story files for many components, removal of multiple thin re-export wrapper files, updates to numerous internal import paths, and a resolver patch in the story importer to map .js/.jsx specifiers to .ts/.tsx. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/context/create-context.ts (1)
25-33:⚠️ Potential issue | 🟡 MinorRemove stale
Spinneroverride claim fromCreateContextOptionsdocs.Line 30-Line 31 still document direct
Spinnerinjection, but this option was removed fromCreateContextOptions. The API docs are now inaccurate.🛠️ Suggested doc fix
- * custom {`@link` Log}, {`@link` Prompts}, {`@link` Status}, and {`@link` Spinner} - * implementations; when omitted, default `@clack/prompts`-backed instances + * custom {`@link` Log}, {`@link` Prompts}, and {`@link` Status} + * implementations; when omitted, default `@clack/prompts`-backed instances🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/context/create-context.ts` around lines 25 - 33, The JSDoc for CreateContextOptions incorrectly claims a direct Spinner override; update the documentation in create-context.ts (the CreateContextOptions docs used by createContext / CommandContext) to remove the stale "Spinner" injection mention and only list the actual supported overrides (e.g., Log, Prompts, Status), ensuring the doc text matches the current CreateContextOptions shape and available overrides.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/designs/component-library.md`:
- Line 77: Add language identifiers to all fenced code blocks in the component
library doc (the directory structure and visual example blocks) so linting
recognizes them; specifically update each triple-backtick block (the directory
structure example and other plaintext examples around the visual examples) to
start with ```text instead of ```; ensure every fenced block between the noted
examples includes the `text` language tag.
In `@packages/core/src/ui/display/alert.tsx`:
- Line 79: The component coerces ReactNode to a string via String(children)
which yields "[object Object]" for JSX; change the prop type so
AlertProps.children is string (not ReactNode), update any usages/prop types that
reference AlertProps and the component signature, and then use children directly
when building contentStr (referencing contentStr and the component that reads
AlertProps.children) to ensure only text is accepted and rendered correctly.
In `@packages/core/src/ui/display/progress-bar.tsx`:
- Line 67: The calculation of ratio in the ProgressBar component can divide by
zero; update the ratio computation (the const ratio in progress-bar.tsx /
ProgressBar) to guard against max === 0 (or non-positive max) before
dividing—e.g., if max is zero or <= 0 treat ratio as 0 (or clamp appropriately),
otherwise compute Math.min(1, Math.max(0, value / max)); apply the same guard
anywhere else that uses value/max.
In `@packages/core/src/ui/display/status-message.tsx`:
- Line 59: The Text element in StatusMessage is coercing children with
String(children) which breaks ReactNode rendering; update the Text rendering in
the StatusMessage component to output children as a ReactNode instead of
string-coercing it (preserve the intended leading space by rendering an explicit
space node or spacing prop and then render children directly). Locate the Text
line that currently reads <Text>{` ${String(children)}`}</Text> and replace it
so it renders a literal space (if needed) and the children ReactNode without
calling String(), ensuring complex children (JSX, elements) render correctly.
In `@packages/core/src/ui/prompts/autocomplete.tsx`:
- Around line 131-139: The Delete key is currently treated the same as
Backspace; update the key handling so key.backspace removes the character before
the cursor (keep using removeCharAt(search, cursorOffset - 1), decrement
cursorOffset and setFocusIndex(0)), while key.delete removes the character at
the cursor (call removeCharAt(search, cursorOffset) only when cursorOffset <
search.length, do not decrement cursorOffset, and still call setSearch and
setFocusIndex(0)); ensure both branches return after handling. Reference the
current handlers around key.backspace / key.delete, removeCharAt, setSearch,
setCursorOffset, and setFocusIndex.
In `@packages/core/src/ui/prompts/group-multi-select.tsx`:
- Around line 245-255: The moveFocus function can land on a disabled option at
the list boundary; update moveFocus(current: number, direction: number, items:
readonly FlatItem[]) to scan in the given direction until it finds an enabled
option and return that index, but if the scan reaches out-of-bounds without
finding any enabled option, return the original current index instead of the
last in-bounds index. Use the existing symbols (current, direction, items,
FlatItem, item.kind === 'option' and item.option.disabled) to implement a loop
that advances next while skipping disabled options and stops with current if no
enabled candidate is found.
In `@packages/core/src/ui/prompts/multi-select.tsx`:
- Around line 141-151: The OptionRow elements use option.label as the React key
which can collide; update the key in the map that renders OptionRow (the block
passing option, indicator, isFocused, isSelected, isDisabled) to use a stable
unique identifier such as option.value (or if value may not be unique, a
combined key like `${option.value}-${index}` or an explicit id field) instead of
option.label so React reconciliation is deterministic; locate the map that
renders OptionRow (references: OptionRow, option.label, focusedIndex) and
replace the key accordingly.
In `@packages/core/src/ui/prompts/navigation.ts`:
- Around line 98-110: The exported function resolveInitialIndex currently
accepts two positional parameters; change its signature to use a single object
parameter with destructured properties (e.g., { options, defaultValue }: {
options: readonly PromptOption<TValue>[], defaultValue?: TValue }) and update
all callers (such as select.tsx where it's invoked) to pass an object instead of
two args; preserve the generic TValue and return type and ensure type
annotations for PromptOption and optional defaultValue remain correct.
---
Outside diff comments:
In `@packages/core/src/context/create-context.ts`:
- Around line 25-33: The JSDoc for CreateContextOptions incorrectly claims a
direct Spinner override; update the documentation in create-context.ts (the
CreateContextOptions docs used by createContext / CommandContext) to remove the
stale "Spinner" injection mention and only list the actual supported overrides
(e.g., Log, Prompts, Status), ensuring the doc text matches the current
CreateContextOptions shape and available overrides.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6ee727fa-2e1a-4614-ac26-6d045471dfe4
⛔ Files ignored due to path filters (1)
.changeset/reorganize-ui-components.mdis excluded by!.changeset/**
📒 Files selected for processing (81)
docs/designs/component-library.mdexamples/diagnostic-output/package.jsonpackages/core/src/cli.tspackages/core/src/context/create-context.tspackages/core/src/index.tspackages/core/src/runtime/runtime.tspackages/core/src/runtime/types.tspackages/core/src/screen/index.tspackages/core/src/screen/output/index.tspackages/core/src/screen/output/screen-log.tspackages/core/src/screen/output/screen-report.tspackages/core/src/screen/output/screen-spinner.tspackages/core/src/screen/output/store-key.tspackages/core/src/screen/output/store.tspackages/core/src/screen/output/types.tspackages/core/src/screen/output/use-output-store.tspackages/core/src/screen/provider.tsxpackages/core/src/screen/screen.test.tspackages/core/src/screen/screen.tsxpackages/core/src/stories/decorators.tsxpackages/core/src/stories/viewer/components/field-control.tsxpackages/core/src/stories/viewer/components/preview.tsxpackages/core/src/stories/viewer/components/props-editor.tsxpackages/core/src/stories/viewer/components/sidebar.tsxpackages/core/src/stories/viewer/stories-app.tsxpackages/core/src/stories/viewer/stories-check.tsxpackages/core/src/test/context.test.tspackages/core/src/test/context.tspackages/core/src/test/types.tspackages/core/src/types/cli.tspackages/core/src/ui/confirm.tsxpackages/core/src/ui/display/alert.stories.tsxpackages/core/src/ui/display/alert.tsxpackages/core/src/ui/display/error-message.tsxpackages/core/src/ui/display/index.tspackages/core/src/ui/display/progress-bar.stories.tsxpackages/core/src/ui/display/progress-bar.tsxpackages/core/src/ui/display/spinner.stories.tsxpackages/core/src/ui/display/spinner.tsxpackages/core/src/ui/display/status-message.stories.tsxpackages/core/src/ui/display/status-message.tsxpackages/core/src/ui/index.tspackages/core/src/ui/layout/fullscreen.test.tspackages/core/src/ui/layout/fullscreen.tsxpackages/core/src/ui/layout/index.tspackages/core/src/ui/layout/scroll-area.tsxpackages/core/src/ui/layout/tabs.tsxpackages/core/src/ui/layout/use-size.test.tspackages/core/src/ui/layout/use-size.tsxpackages/core/src/ui/multi-select.tsxpackages/core/src/ui/output.tsxpackages/core/src/ui/password-input.tsxpackages/core/src/ui/prompts/autocomplete.stories.tsxpackages/core/src/ui/prompts/autocomplete.tsxpackages/core/src/ui/prompts/confirm.stories.tsxpackages/core/src/ui/prompts/confirm.tsxpackages/core/src/ui/prompts/cursor-value.tsxpackages/core/src/ui/prompts/group-multi-select.stories.tsxpackages/core/src/ui/prompts/group-multi-select.tsxpackages/core/src/ui/prompts/index.tspackages/core/src/ui/prompts/input-state.tspackages/core/src/ui/prompts/multi-select.stories.tsxpackages/core/src/ui/prompts/multi-select.tsxpackages/core/src/ui/prompts/navigation.tspackages/core/src/ui/prompts/option-row.tsxpackages/core/src/ui/prompts/password-input.stories.tsxpackages/core/src/ui/prompts/password-input.tsxpackages/core/src/ui/prompts/path-input.stories.tsxpackages/core/src/ui/prompts/path-input.tsxpackages/core/src/ui/prompts/select-key.stories.tsxpackages/core/src/ui/prompts/select-key.tsxpackages/core/src/ui/prompts/select.stories.tsxpackages/core/src/ui/prompts/select.tsxpackages/core/src/ui/prompts/string-utils.tspackages/core/src/ui/prompts/text-input.stories.tsxpackages/core/src/ui/prompts/text-input.tsxpackages/core/src/ui/prompts/types.tspackages/core/src/ui/select.tsxpackages/core/src/ui/spinner.tsxpackages/core/src/ui/text-input.tsxpackages/core/src/ui/theme.ts
💤 Files with no reviewable changes (9)
- packages/core/src/runtime/runtime.ts
- packages/core/src/ui/multi-select.tsx
- packages/core/src/ui/spinner.tsx
- packages/core/src/ui/select.tsx
- packages/core/src/cli.ts
- packages/core/src/ui/text-input.tsx
- packages/core/src/ui/confirm.tsx
- packages/core/src/screen/output/index.ts
- packages/core/src/ui/password-input.tsx
- Add language identifiers to fenced code blocks in component-library.md - Restrict Alert and StatusMessage children to string (prevents [object Object]) - Guard ProgressBar against division by zero when max is 0 - Fix Delete key in Autocomplete to remove char at cursor (not before) - Prevent focus landing on disabled option at boundary in GroupMultiSelect - Use option.value as React key in MultiSelect to avoid collisions - Refactor resolveInitialIndex to use object destructuring per conventions Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
- Add @kidd-cli/cli as root devDependency for direct `kidd stories` access - Patch Module._resolveFilename in story importer to handle TypeScript ESM-style .js -> .ts/.tsx extension mapping (same strategy as tsx/ts-node) - Replace strikethrough on disabled options with gray color (matches clack) - Add (disabled) text fallback when colors are not supported - Use gray + dim for disabled option hints Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/core/src/ui/prompts/select.tsx (1)
115-125:⚠️ Potential issue | 🟠 MajorUse a stable key to avoid reconciliation bugs.
At Line 124,
key={option.label}can collide when labels repeat. That can misrender focus/selection state between rows.Proposed fix
<OptionRow - key={option.label} + key={String(option.value)} option={option} indicator={indicator} isFocused={index === focusedIndex}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/ui/prompts/select.tsx` around lines 115 - 125, The list uses option.label as the React key in the options.map rendering (OptionRow) which can collide for duplicate labels; change the key to a stable unique identifier such as option.id or, if no id exists, use the map index only as a fallback (e.g., key={option.id ?? `option-${index}`}) so each OptionRow has a stable, unique key and avoids reconciliation/focus/selection bugs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/designs/component-library.md`:
- Around line 645-650: The fenced code block containing the dependency graph
(the block with "foundation", "├── prompts-core", "├── prompts-extended", "└──
display") is missing a language identifier; update that fenced block to use the
plaintext language identifier by changing the opening fence from ``` to ```text
so it matches other plaintext examples in the doc.
In `@packages/core/src/ui/prompts/autocomplete.tsx`:
- Around line 103-106: When handling the down-arrow case in the autocomplete key
handler, guard against an empty filtered list so focusIndex isn't set to -1:
check filtered.length > 0 before computing next and calling setFocusIndex; if
filtered is empty, do nothing (or keep focusIndex at -1/0 per desired behavior).
Update the block that uses key.downArrow, Math.min(filtered.length - 1,
focusIndex + 1), setFocusIndex and the focused = filtered[next] access to only
run when filtered.length > 0 to avoid negative indices and out-of-bounds access.
In `@packages/core/src/ui/prompts/group-multi-select.tsx`:
- Around line 332-337: The key generator itemKey for FlatItem can produce
collisions when two options in the same group have identical labels; update
itemKey to use a stable, unique identifier (e.g., option.value) for option keys
instead of option.label, ensuring option.value is stringified if necessary;
change the option branch in itemKey to generate
`option-${groupName}-${String(option.value)}` and verify the FlatItem/option
types (and any generic TValue) support or coerce to a string to avoid runtime
key collisions.
---
Duplicate comments:
In `@packages/core/src/ui/prompts/select.tsx`:
- Around line 115-125: The list uses option.label as the React key in the
options.map rendering (OptionRow) which can collide for duplicate labels; change
the key to a stable unique identifier such as option.id or, if no id exists, use
the map index only as a fallback (e.g., key={option.id ?? `option-${index}`}) so
each OptionRow has a stable, unique key and avoids
reconciliation/focus/selection bugs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f0b29016-2307-4cc0-868c-7d15296e2688
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (17)
codspeed.ymldocs/designs/component-library.mdexamples/tui/package.jsonpackage.jsonpackages/core/src/stories/importer.tspackages/core/src/stories/viewer/components/preview.tsxpackages/core/src/stories/viewer/stories-app.tsxpackages/core/src/ui/display/alert.tsxpackages/core/src/ui/display/progress-bar.tsxpackages/core/src/ui/display/status-message.tsxpackages/core/src/ui/index.tspackages/core/src/ui/prompts/autocomplete.tsxpackages/core/src/ui/prompts/group-multi-select.tsxpackages/core/src/ui/prompts/multi-select.tsxpackages/core/src/ui/prompts/navigation.tspackages/core/src/ui/prompts/option-row.tsxpackages/core/src/ui/prompts/select.tsx
Static test/story data doesn't need to be frozen — it's already a const binding that nothing mutates. Co-Authored-By: Claude <noreply@anthropic.com>
- Remove docs/designs/component-library.md design doc - Guard focus movement in autocomplete when filtered list is empty - Fix key collision in group-multi-select by using option.value instead of label Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/ui/prompts/autocomplete.tsx`:
- Around line 93-104: The up-arrow branch for the autocomplete currently only
clamps the lower bound, so if focusIndex is greater than filtered.length - 1 the
new index stays out of range; update the calculation of next in that branch (the
code that sets focusIndex via setFocusIndex) to clamp both ends using
filtered.length - 1 (e.g., compute next = Math.max(0, Math.min(filtered.length -
1, focusIndex - 1))) and then use that next when calling setFocusIndex and
onChange (referencing focusIndex, filtered, setFocusIndex, and onChange).
In `@packages/core/src/ui/prompts/group-multi-select.tsx`:
- Around line 75-80: The focusIndex state is initialized once and can become
out-of-bounds when flatItems (from buildFlatItems using options and
selectableGroups) changes; add a reconciliation effect that runs when flatItems
changes to clamp or reset focusIndex via setFocusIndex to a valid entry (e.g.,
min(current, flatItems.length - 1)) and prefer the first enabled/selectable item
(skip disabled rows) so keyboard navigation and initial focus never point at a
removed or disabled item; reference the flatItems variable,
focusIndex/setFocusIndex state, and the buildFlatItems/selectableGroups logic
when implementing this fix.
- Around line 84-88: The early return in the useInput handler prevents Enter
handling when flatItems is empty; update the handler in useInput (the callback
referencing flatItems) so it does not unconditionally return on flatItems.length
=== 0 — instead only skip navigation/selection logic for empty lists but still
allow the Enter/Return branch to run so non-required prompts can submit [] and
required prompts can run validation; adjust the conditional to check key.name
(e.g., 'return'/'enter') and only bypass other key handling when flatItems is
empty while preserving Enter processing.
- Around line 90-122: Replace the sequential if-returns in the input handler
with a ts-pattern match() dispatch so control flow is explicit and conforms to
the style guide: use match({ key, input }) (or match(key) and match on input
where appropriate) to branch on upArrow, downArrow, input === ' ' (space) and
return; in each arm call the same side-effecting helpers
(setFocusIndex(moveFocus(...)), toggleItem -> setSelected -> setError(undefined)
-> onChange, and onSubmit) and preserve the required validation (required &&
selected.length === 0) and early returns inside the matching arm; update imports
to include match from 'ts-pattern' and keep references to setFocusIndex,
moveFocus, toggleItem, setSelected, onChange, onSubmit unchanged so the behavior
remains identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7de35f6a-2277-4ead-adf9-0f09866ebbf6
📒 Files selected for processing (2)
packages/core/src/ui/prompts/autocomplete.tsxpackages/core/src/ui/prompts/group-multi-select.tsx
Co-Authored-By: Claude <noreply@anthropic.com>
- Clamp focusIndex on upArrow in autocomplete when filtered list shrinks - Sync focusIndex with flatItems length in group-multi-select via useEffect - Move Enter handling before empty list guard so non-required prompts can submit [] Co-Authored-By: Claude <noreply@anthropic.com>
|
Re: using |
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (4)
packages/core/src/ui/prompts/autocomplete.tsx (1)
83-92:⚠️ Potential issue | 🟠 MajorUp-arrow still lacks upper-bound clamping for
focusIndex.Line 87 only clamps the lower bound. If
focusIndexis greater thanfiltered.length - 1, the index stays out of range and focus behavior degrades until multiple key presses recover it.Proposed fix
- const next = Math.max(0, focusIndex - 1) + const next = Math.max(0, Math.min(filtered.length - 1, focusIndex - 1))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/ui/prompts/autocomplete.tsx` around lines 83 - 92, The up-arrow handler in the autocomplete (code using focusIndex, filtered, setFocusIndex) only clamps the lower bound and can operate on an out-of-range focusIndex when focusIndex > filtered.length-1; before computing the new index, first clamp the current focusIndex to Math.min(focusIndex, filtered.length - 1) (or set a local boundedIndex = Math.min(focusIndex, filtered.length - 1)), then compute next = Math.max(0, boundedIndex - 1), call setFocusIndex(next) and use filtered[next] for onChange to guarantee the index is always within [0, filtered.length-1].packages/core/src/ui/prompts/group-multi-select.tsx (3)
74-78:⚠️ Potential issue | 🟠 MajorLet Enter run even when there are no rows.
Lines 76-78 return before the Enter branch executes. A non-required prompt can never submit
[], and a required prompt never surfaces its validation error whenflatItemsis empty.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/ui/prompts/group-multi-select.tsx` around lines 74 - 78, The current early return in the useInput handler prevents the Enter key branch from running when flatItems is empty; update the handler in group-multi-select's useInput callback so the Enter key is checked and handled before returning for empty flatItems (or remove the early return and guard only branches that require items). Concretely, in the useInput ((input, key) => { ... }) callback, check key.name === "return" first and allow submission logic to run even if flatItems.length === 0, and keep the empty-flatItems guard only around navigation/selection logic that depends on flatItems.
65-71:⚠️ Potential issue | 🟠 MajorClamp
focusIndexwhenflatItemschanges.Line 69 seeds
focusIndexonce and never reconciles it afteroptionsorselectableGroupschange. If rows are removed, or the first row becomes disabled, focus can stay out of bounds or land on a disabled item and keyboard navigation cannot recover until remount.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/ui/prompts/group-multi-select.tsx` around lines 65 - 71, The focusIndex is only initialized once and can become out-of-bounds or point to a disabled row when flatItems (built via buildFlatItems from options/selectableGroups) changes; add a useEffect that runs when flatItems changes and clamps/adjusts focusIndex via setFocusIndex so it stays within 0..flatItems.length-1 and, if the current target is disabled, moves focus to the nearest enabled item (forward or backward) or to -1/0 as your component expects; reference focusIndex, setFocusIndex, flatItems, and buildFlatItems when locating where to add this reconciliation.
80-112:⚠️ Potential issue | 🟠 MajorReplace the multi-branch input dispatch with
match().This handler dispatches up/down/space/Enter with sequential
ifreturns. That is multi-branch domain logic, so it falls under the repo’s requiredts-patternstyle instead of the documented guard-clause exemptions.Based on learnings, only simple type-narrowing guards and Result-tuple early returns are exempt from the
match()rule incontributing/standards/typescript/coding-style.md.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/ui/prompts/group-multi-select.tsx` around lines 80 - 112, Replace the sequential if-return input dispatch with a ts-pattern match on the input/key combination: locate the handler using variables key, input, focusIndex, flatItems and functions setFocusIndex/moveFocus, toggleItem/setSelected/setError, onChange, onSubmit; implement match() (from ts-pattern) to handle cases for upArrow (moveFocus -1), downArrow (moveFocus +1), space (toggleItem => setSelected, clear error, call onChange), and return/enter (validate required -> setError or call onSubmit), preserving all existing behavior and early returns but expressed as match arms.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/ui/display/alert.tsx`:
- Around line 182-189: resolveInnerWidth incorrectly measures content using
contentStr.length and a single title length, causing borders from buildTopBorder
and alignLine to misalign for multiline content or when a provided numeric width
is too small; update resolveInnerWidth (and its use of
InnerWidthOptions/title/contentStr) to compute the effective inner width by
splitting contentStr on newlines, taking the maximum length of all content
lines, computing titleWidth as before, and then returning
Math.max(maxContentLineLength, titleWidth, 0) (while still applying the width
!== 'auto' branch: subtract the fixed padding from the provided width but clamp
it to at least that computed max), so top/bottom borders and alignLine will use
a consistent inner width.
In `@packages/core/src/ui/display/progress-bar.tsx`:
- Around line 62-66: Guard against negative or non-integer size before calling
String.repeat by validating and normalizing size and using that normalized value
when computing filledCount and emptyCount; e.g., in the progress bar logic where
size, filledCount, emptyCount, resolveChars(style), chars.filled.repeat and
chars.empty.repeat are used, clamp size to a non-negative integer (e.g., size =
Math.max(0, Math.floor(size))), then compute filledCount = Math.round(ratio *
size) and emptyCount = size - filledCount so both repeats receive non-negative
counts and cannot throw RangeError.
In `@packages/core/src/ui/display/spinner.tsx`:
- Around line 107-128: The spinner can briefly render undefined when `type`
changes because `frameIndex` may be out of bounds for the new `spinner.frames`;
update the component to reset `frameIndex` whenever the spinner `type` (or
`spinner.frames.length`) changes by calling `setFrameIndex(0)` (or clamp to the
new length) in an effect that depends on `type` or `spinner.frames.length` so
`spinner.frames[frameIndex]` is always valid; locate the `frameIndex` state and
the existing useEffect that advances frames and add a new useEffect (or expand
the dependency handling) to reset/clamp `frameIndex` when the spinner identity
or frames length changes.
In `@packages/core/src/ui/display/status-message.tsx`:
- Around line 65-72: resolveIcon currently pattern-matches StatusMessageVariant
but omits the 'primary' variant, so .exhaustive() will throw at runtime for
variant="primary"; update the resolveIcon function to handle 'primary' (e.g.,
add .with('primary', () => symbols.circle) or another appropriate symbols.*
value) so all Variant/StatusMessageVariant cases are covered and the match
remains exhaustive.
In `@packages/core/src/ui/prompts/navigation.ts`:
- Around line 66-79: The function resolveFirstEnabledIndex is exported but sits
under the "// Private" section header; either move the
resolveFirstEnabledIndex<TValue> export out of the private block into the "//
Exports" section (or another appropriate exported section) or change the section
header text to accurately reflect that exported symbols live there — update the
comment near the top that currently reads "// Private" (and any adjacent section
dividers) so the exported function resolveFirstEnabledIndex is grouped under the
correct section header.
In `@packages/core/src/ui/prompts/path-input.tsx`:
- Around line 1-2: The component currently uses blocking node:fs APIs
readdirSync and statSync (e.g., in PathInput) which can freeze the event loop;
replace them with the async counterparts (readdir, stat) and perform directory
reads inside a useEffect or an async handler with proper cancellation/abort
logic, update state with debouncing to avoid spamming reads during fast typing,
and ensure errors are caught and mapped to empty suggestions so the UI stays
responsive during tab completion.
In `@packages/core/src/ui/prompts/select-key.tsx`:
- Around line 16-17: The component SelectKey is rendering only the first
character of PromptOption.value but comparing user input against the full
option.value, causing mismatches for multi-character values; update SelectKey so
it consistently uses the first character of each option for both display and
input matching (e.g., compute const keyChar = option.value[0] and use keyChar in
the rendered key label and in the input comparison) or alternatively
validate/normalize options to single-character values up-front (throw or filter
inside SelectKey) so display and selection logic (options, option.value, and the
input-match code path) remain consistent.
In `@packages/core/src/ui/prompts/select.tsx`:
- Around line 112-114: OptionRow currently uses option.label as the React key
which can collide for duplicate labels; change the key to a stable unique
identifier (e.g., option.id or option.value) on the OptionRow element and ensure
the options array supplies that stable id (or generate a stable id when options
are constructed). Update the OptionRow usage (the JSX that renders <OptionRow
key={...} option={option} />) to reference the unique id field and, if no such
field exists in the options data, add one where options are created (e.g., add
an id property) rather than relying on label or array index.
In `@packages/core/src/ui/prompts/string-utils.ts`:
- Around line 12-14: Change the exported functions removeCharAt and insertCharAt
to accept a single destructured parameter object (e.g., function removeCharAt({
str, index }: { str: string; index: number })) instead of positional params,
update their implementations to use the named properties, and then update all
call sites (notably in path-input.tsx) to pass an object with the appropriate
keys (str, index and for insertCharAt any additional param like char) so the
functions conform to the project rule requiring object destructuring for
exported functions with 2+ parameters.
---
Duplicate comments:
In `@packages/core/src/ui/prompts/autocomplete.tsx`:
- Around line 83-92: The up-arrow handler in the autocomplete (code using
focusIndex, filtered, setFocusIndex) only clamps the lower bound and can operate
on an out-of-range focusIndex when focusIndex > filtered.length-1; before
computing the new index, first clamp the current focusIndex to
Math.min(focusIndex, filtered.length - 1) (or set a local boundedIndex =
Math.min(focusIndex, filtered.length - 1)), then compute next = Math.max(0,
boundedIndex - 1), call setFocusIndex(next) and use filtered[next] for onChange
to guarantee the index is always within [0, filtered.length-1].
In `@packages/core/src/ui/prompts/group-multi-select.tsx`:
- Around line 74-78: The current early return in the useInput handler prevents
the Enter key branch from running when flatItems is empty; update the handler in
group-multi-select's useInput callback so the Enter key is checked and handled
before returning for empty flatItems (or remove the early return and guard only
branches that require items). Concretely, in the useInput ((input, key) => { ...
}) callback, check key.name === "return" first and allow submission logic to run
even if flatItems.length === 0, and keep the empty-flatItems guard only around
navigation/selection logic that depends on flatItems.
- Around line 65-71: The focusIndex is only initialized once and can become
out-of-bounds or point to a disabled row when flatItems (built via
buildFlatItems from options/selectableGroups) changes; add a useEffect that runs
when flatItems changes and clamps/adjusts focusIndex via setFocusIndex so it
stays within 0..flatItems.length-1 and, if the current target is disabled, moves
focus to the nearest enabled item (forward or backward) or to -1/0 as your
component expects; reference focusIndex, setFocusIndex, flatItems, and
buildFlatItems when locating where to add this reconciliation.
- Around line 80-112: Replace the sequential if-return input dispatch with a
ts-pattern match on the input/key combination: locate the handler using
variables key, input, focusIndex, flatItems and functions
setFocusIndex/moveFocus, toggleItem/setSelected/setError, onChange, onSubmit;
implement match() (from ts-pattern) to handle cases for upArrow (moveFocus -1),
downArrow (moveFocus +1), space (toggleItem => setSelected, clear error, call
onChange), and return/enter (validate required -> setError or call onSubmit),
preserving all existing behavior and early returns but expressed as match arms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f5f83e09-6606-44ff-a5f0-1ecd3cc3108a
📒 Files selected for processing (55)
packages/core/src/context/prompts.tspackages/core/src/context/resolve-defaults.tspackages/core/src/context/status.tspackages/core/src/lib/log.tspackages/core/src/middleware/auth/oauth-server.tspackages/core/src/middleware/icons/definitions.tspackages/core/src/middleware/icons/icons.tspackages/core/src/middleware/report/report.tspackages/core/src/middleware/report/types.tspackages/core/src/screen/index.tspackages/core/src/screen/output/index.tspackages/core/src/screen/output/screen-log.tspackages/core/src/screen/output/screen-report.tspackages/core/src/screen/output/screen-spinner.tspackages/core/src/screen/output/store-key.tspackages/core/src/screen/output/store.tspackages/core/src/screen/output/types.tspackages/core/src/screen/output/use-output-store.tspackages/core/src/stories/index.tspackages/core/src/stories/viewer/hooks/use-double-escape.tspackages/core/src/stories/viewer/utils.tspackages/core/src/ui/display/alert.tsxpackages/core/src/ui/display/error-message.tsxpackages/core/src/ui/display/index.tspackages/core/src/ui/display/progress-bar.tsxpackages/core/src/ui/display/spinner.tsxpackages/core/src/ui/display/status-message.tsxpackages/core/src/ui/index.tspackages/core/src/ui/input-barrier.tsxpackages/core/src/ui/keys.tspackages/core/src/ui/layout/fullscreen.tsxpackages/core/src/ui/layout/index.tspackages/core/src/ui/layout/scroll-area.tsxpackages/core/src/ui/layout/tabs.tsxpackages/core/src/ui/layout/use-size.tsxpackages/core/src/ui/output.tsxpackages/core/src/ui/prompts/autocomplete.tsxpackages/core/src/ui/prompts/confirm.tsxpackages/core/src/ui/prompts/cursor-value.tsxpackages/core/src/ui/prompts/group-multi-select.tsxpackages/core/src/ui/prompts/index.tspackages/core/src/ui/prompts/input-state.tspackages/core/src/ui/prompts/multi-select.tsxpackages/core/src/ui/prompts/navigation.tspackages/core/src/ui/prompts/option-row.tsxpackages/core/src/ui/prompts/password-input.tsxpackages/core/src/ui/prompts/path-input.tsxpackages/core/src/ui/prompts/select-key.tsxpackages/core/src/ui/prompts/select.tsxpackages/core/src/ui/prompts/string-utils.tspackages/core/src/ui/prompts/text-input.tsxpackages/core/src/ui/prompts/types.tspackages/core/src/ui/theme.tspackages/core/src/ui/use-key-binding.tspackages/core/src/ui/use-key-input.ts
💤 Files with no reviewable changes (25)
- packages/core/src/stories/viewer/utils.ts
- packages/core/src/screen/output/screen-report.ts
- packages/core/src/screen/output/types.ts
- packages/core/src/screen/output/use-output-store.ts
- packages/core/src/screen/output/screen-log.ts
- packages/core/src/context/status.ts
- packages/core/src/ui/layout/scroll-area.tsx
- packages/core/src/ui/use-key-binding.ts
- packages/core/src/middleware/icons/icons.ts
- packages/core/src/context/resolve-defaults.ts
- packages/core/src/middleware/auth/oauth-server.ts
- packages/core/src/lib/log.ts
- packages/core/src/screen/output/index.ts
- packages/core/src/screen/output/screen-spinner.ts
- packages/core/src/ui/layout/use-size.tsx
- packages/core/src/context/prompts.ts
- packages/core/src/middleware/icons/definitions.ts
- packages/core/src/middleware/report/types.ts
- packages/core/src/ui/keys.ts
- packages/core/src/ui/use-key-input.ts
- packages/core/src/stories/index.ts
- packages/core/src/stories/viewer/hooks/use-double-escape.ts
- packages/core/src/ui/input-barrier.tsx
- packages/core/src/middleware/report/report.ts
- packages/core/src/screen/output/store.ts
- Fix alert width calculation for multiline content (use max line width) - Guard progress bar repeat counts with Math.max(0) for negative size - Reset spinner frameIndex when type changes to prevent out-of-bounds - Move Private section separator to correct position in navigation.ts - Make select-key matching case-insensitive - Use String(option.value) for stable React keys in select - Convert string-utils to object destructuring per project conventions Co-Authored-By: Claude <noreply@anthropic.com>
|
Re: status-message.tsx non-exhaustive match (PRRT_kwDORa2ehc53kNKg) The |
|
Re: path-input.tsx sync filesystem ops (PRRT_kwDORa2ehc53kNKj) The sync filesystem operations are intentional. This is a CLI prompt for navigating local directories — the operations run on small local directories during interactive user input. The simplicity of synchronous reads outweighs the marginal benefit of async I/O in a CLI context, where making these async would require useEffect + loading states for no real-world benefit. |
Summary
prompts/,display/, andlayout/subdirectoriesAutocomplete,GroupMultiSelect,PathInput,SelectKeyAlert,ProgressBar,Spinner,StatusMessagescreen/module fromui/with provider, context, and output storeoutput.tsxfileTest plan
pnpm checkpasses (typecheck + lint + format)pnpm testpasses